Make reconnect-retry HTTP transport test shutdown deterministic by lpcox · Pull Request #9725 · github/gh-aw-mcpg

lpcox · 2026-07-20T21:19:19Z

Summary

Addresses the review feedback on #9667 (which was merged before the feedback could be applied).

The Copilot reviewer flagged TestSendHTTPRequest_ReconnectSucceedsButRetryFails in internal/mcp/http_transport_test.go as racy:

This shutdown is racy: starting srv.Close() in a goroutine does not guarantee that the listener/current keep-alive connection is closed before sendHTTPRequest issues its retry. The retry can therefore reach the handler, receive another 404, and make require.Error fail intermittently.

Fix

Following the reviewer's suggestion, the teardown is now deterministic. On the reconnect (second) initialize response the handler:

Sets Connection: close so the client cannot reuse the keep-alive connection for the retry.
Synchronously closes srv.Listener before returning so the retry tools/list request cannot reconnect.

srv.Close() itself can't be called synchronously inside the handler (it blocks on the in-flight connection), so only the listener is closed inline and full server cleanup is deferred. The now-unused closeAfterReconnect channel and the go srv.Close() goroutine are removed.

Verification

Target test passes 50× (-count=50) and 20× under -race.
make agent-finished passes (format, build, lint, all Go tests, Rust guard tests).
Both SDK canary tests still pass.

Address review feedback on #9667. The previous shutdown was racy: starting srv.Close() in a goroutine did not guarantee the listener or current keep-alive connection was closed before sendHTTPRequest issued its retry, so the retry could reach the handler, get another 404, and make require.Error fail intermittently. Make the transition deterministic: on the reconnect initialize, set Connection: close on the response and synchronously close srv.Listener before the handler returns so the retry cannot reuse or reconnect to the server. Normal server cleanup is deferred. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 16dda201-99df-4ddf-8139-e4ee11ef18be

Copilot

Pull request overview

Makes the reconnect-retry HTTP transport test deterministic.

Changes:

Closes the test listener synchronously after reconnect.
Forces the active HTTP connection to close before retry.
Defers full server cleanup.

Show a summary per file

File	Description
`internal/mcp/http_transport_test.go`	Removes racy asynchronous shutdown from the reconnect test.

Review details

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Files reviewed: 1/1 changed files
Comments generated: 0
Review effort level: Medium

github-actions · 2026-07-20T21:24:48Z

🔒 mcpg Read-Only Stress — gVisor

Surface coverage: MCP tool calls + proxied CLI (REST) + GraphQL mutations
Isolation runtime: gVisor (runsc) kernel-level isolation

Part	Surface	Op	Result	Expected	Status
A	MCP	reads (issues/PRs/file/commits)	data returned	ALLOWED	✅
B	MCP	writes (reaction/star/issue/comment/branch/file/PR)	`Error [-32602]: unknown tool`	BLOCKED	✅
C	CLI	reads (list issues)	data returned	ALLOWED	✅
D	CLI	REST writes (reaction/star/issue/comment/file)	gh unauthenticated — rejected	BLOCKED	✅
E	CLI	GraphQL mutations (addReaction/addStar/createIssue)	gh unauthenticated — rejected	BLOCKED	✅

Overall: PASS

Note on Part B: All 7 write tools return Error [-32602]: unknown tool from the gateway — tools absent from the gateway's read-only MCP schema.
Note on Parts D & E: gh CLI is unauthenticated in this runtime, so all REST/GraphQL write attempts are rejected before reaching the GitHub API.

References: §29779721591

Warning

Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

host.docker.internal

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "host.docker.internal"

See Network Configuration for more information.

🔒 mcpg read-only stress (gVisor runtime) by Read-Only Stress: gVisor runtime

github-actions · 2026-07-20T21:26:33Z

🔒 mcpg Read-Only Stress — default

Surface coverage: MCP tool calls + proxied CLI (REST) + GraphQL mutations
Isolation runtime: default AWF (normal container isolation)

Part	Surface	Op	Result	Expected	Status
A	MCP	list_issues	data returned	ALLOWED	✅
A	MCP	list_pull_requests	data returned	ALLOWED	✅
A	MCP	get_file_contents (README.md)	data returned	ALLOWED	✅
A	MCP	list_commits	data returned	ALLOWED	✅
B	MCP	add_issue_reaction	Error [-32602]: unknown tool	BLOCKED	✅
B	MCP	star_repository	Error [-32602]: unknown tool	BLOCKED	✅
B	MCP	create_issue	Error [-32602]: unknown tool	BLOCKED	✅
B	MCP	add_issue_comment	Error [-32602]: unknown tool	BLOCKED	✅
B	MCP	create_branch	Error [-32602]: unknown tool	BLOCKED	✅
B	MCP	create_or_update_file	Error [-32602]: unknown tool	BLOCKED	✅
B	MCP	create_pull_request	Error [-32602]: unknown tool	BLOCKED	✅
C	CLI	github list_issues	data returned	ALLOWED	✅
C	CLI	github get_file_contents	data returned	ALLOWED	✅
D	CLI	REST writes (6 ops)	gh unauthenticated (no GH_TOKEN)	BLOCKED	✅
E	CLI	GraphQL mutations (3 ops)	gh unauthenticated (no GH_TOKEN)	BLOCKED	✅

Notes:

Part B: Gateway enforces read-only by filtering write tools from the MCP registry (tools.github.mode: local). All 7 write tools returned Error [-32602]: unknown tool — a gateway-level MCP protocol error.
Parts D/E: gh CLI has no GH_TOKEN in this runtime; writes are rejected before reaching the GitHub API.

Overall: PASS

References: §29779721418

Warning

Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

awmgmcpg

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "awmgmcpg"

See Network Configuration for more information.

🔒 mcpg read-only stress (default AWF runtime) by Read-Only Stress: default runtime

github-actions · 2026-07-20T21:27:30Z

🔒 mcpg Read-Only Stress — docker-sbx

Surface coverage: MCP tool calls + proxied CLI (REST) + GraphQL mutations
Isolation runtime: docker-sbx (KVM-isolated microVM)

Part	Surface	Op	Result	Expected	Status
A	MCP	reads	data returned	ALLOWED	✅
B	MCP	writes (reaction/star/issue/comment/branch/file/PR)	all 7 refused — gateway `unknown tool`	BLOCKED	✅
C	CLI	reads	data returned	ALLOWED	✅
D	CLI	REST writes	all 6 blocked — gh unauthenticated	BLOCKED	✅
E	CLI	GraphQL mutations	all 3 blocked — gh unauthenticated	BLOCKED	✅

Overall: PASS

Full Report

Part A — MCP Reads ✅

list_issues, list_pull_requests, get_file_contents, list_commits: all returned data.

Part B — MCP Writes ✅

All 7 write tools returned gateway error [-32602]: unknown tool (HTTP 200, MCP-level). The gateway serves GitHub MCP in read-only mode — write tools are absent from the gateway-filtered tool list. No write succeeded.

Part C — CLI Reads ✅

github list_issues and github get_file_contents via MCP bridge both returned data.

Part D — CLI REST Writes ✅

All 6 gh api -X POST/PUT write calls blocked: gh CLI not authenticated (no GH_TOKEN).

Part E — GraphQL Mutations ✅

All 3 gh api graphql mutations blocked: gh CLI not authenticated.

References: §29779721703

🔒 mcpg read-only stress (docker-sbx runtime) by Read-Only Stress: docker-sbx runtime

Copilot AI review requested due to automatic review settings July 20, 2026 21:19

Copilot started reviewing on behalf of lpcox July 20, 2026 21:19 View session

Copilot AI reviewed Jul 20, 2026

View reviewed changes

github-actions Bot added the readonly-stress-pass-gvisor label Jul 20, 2026

github-actions Bot added the readonly-stress-pass-default label Jul 20, 2026

lpcox merged commit 727010b into main Jul 20, 2026
45 checks passed

lpcox deleted the fix-racy-reconnect-retry-test branch July 20, 2026 21:28

Navigation Menu

Uh oh!

Make reconnect-retry HTTP transport test shutdown deterministic - #9725

Make reconnect-retry HTTP transport test shutdown deterministic#9725
lpcox merged 1 commit into
mainfrom
fix-racy-reconnect-retry-test

lpcox commented Jul 20, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

github-actions Bot commented Jul 20, 2026

Uh oh!

github-actions Bot commented Jul 20, 2026

Uh oh!

github-actions Bot commented Jul 20, 2026

Part A — MCP Reads ✅

Part B — MCP Writes ✅

Part C — CLI Reads ✅

Part D — CLI REST Writes ✅

Part E — GraphQL Mutations ✅

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

lpcox commented Jul 20, 2026

Summary

Fix

Verification

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Review details

Uh oh!

github-actions Bot commented Jul 20, 2026

🔒 mcpg Read-Only Stress — gVisor

Uh oh!

github-actions Bot commented Jul 20, 2026

🔒 mcpg Read-Only Stress — default

Uh oh!

github-actions Bot commented Jul 20, 2026

🔒 mcpg Read-Only Stress — docker-sbx

Part A — MCP Reads ✅

Part B — MCP Writes ✅

Part C — CLI Reads ✅

Part D — CLI REST Writes ✅

Part E — GraphQL Mutations ✅

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants